Preserve input order for grouped window evaluation by rjzamora · Pull Request #23466 · NVIDIA/cudf

rjzamora · 2026-07-28T21:07:04Z

Description

Follow up to #23451
Fully addresses item (7) of #22032

Adds the necessary per-group sort operation to preserve input row order before evaluating input-order-sensitive over(...) expressions without an explicit order_by argument.

This allows streaming execution of grouped shift and cum_sum windows that rely on the incoming row order.

Checklist

I am familiar with the Contributing Guidelines.
New or existing tests cover these changes.
The documentation is up to date with these changes.

coderabbitai · 2026-07-28T21:10:12Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0c53ceb3-d384-4798-a337-5151bc2c6759

📥 Commits

Reviewing files that changed from the base of the PR and between 7ea279c and 6ca710a.

📒 Files selected for processing (1)

python/cudf_polars/cudf_polars/streaming/actor_graph/over.py

🚧 Files skipped from review as they are similar to previous changes (1)

python/cudf_polars/cudf_polars/streaming/actor_graph/over.py

📝 Walkthrough

Summary by CodeRabbit

Bug Fixes
- Improved over() window evaluation in multi-partition streaming by preserving required input order via a stable, input-order-aware sort of stamped rows before computing window expressions.
- Updated selection/lowering behavior to no longer fall back to an unsupported path for input-order-sensitive window expressions lacking order_by.
Tests
- Refreshed streaming SPMD coverage to verify correct multi-rank results for shift(1).over(...) and cum_sum().over(...) (no longer expecting an error), with deterministic global ordering.

Walkthrough

Changes

Multirank streaming over() evaluation now conditionally restores input order before evaluating applicable windows. The previous Select fallback is removed, and SPMD tests cover shift and cum_sum without order_by.

Multirank window ordering

Layer / File(s)	Summary
Stamped row ordering and evaluation `python/cudf_polars/cudf_polars/streaming/actor_graph/over.py`	Forward-shuffled stamped rows are optionally stably sorted by origin rank and chunk order before window evaluation.
Select lowering and multirank validation `python/cudf_polars/cudf_polars/streaming/select.py`, `python/cudf_polars/tests/streaming/test_spmd.py`	The multirank input-order window fallback is removed, and tests validate `shift` and `cum_sum` results without `order_by`.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

rapidsai/cudf#23206: Related multirank streaming handling for over() windows, including cum_sum fallback behavior.
rapidsai/cudf#23306: Related changes to over() partitioning and fast-path decisions.
rapidsai/cudf#23451: Related multirank support for implicit input-order window operations.

Suggested labels: improvement

Suggested reviewers: tomaugspurger

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: preserving input order for grouped window evaluation.
Description check	✅ Passed	The description is directly related to the changeset and describes the input-order-preserving window evaluation update.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@python/cudf_polars/tests/streaming/test_spmd.py`:
- Around line 597-602: Strengthen the test around local_result in the SPMD
collection flow by asserting each rank owns the expected rows in the expected
local order before calling allgather_polars_dataframe. Add rank-aware ownership
checks using the test’s existing distributed context, then retain the global
sorted assertion for the gathered result.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2318405c-4ef3-4cb4-b28c-26ad24613dc7

📥 Commits

Reviewing files that changed from the base of the PR and between f38653b and b4d9ed4.

📒 Files selected for processing (3)

python/cudf_polars/cudf_polars/streaming/actor_graph/over.py
python/cudf_polars/cudf_polars/streaming/select.py
python/cudf_polars/tests/streaming/test_spmd.py

💤 Files with no reviewable changes (1)

python/cudf_polars/cudf_polars/streaming/select.py

rjzamora · 2026-07-29T02:39:51Z

/merge

Follow-up to #23466 That PR introduced support for input-order-sensitive `over(...)` expressions without an explicit `order_by` argument. However, there is a bug: We do not properly sort by the original local row-position (only the rank and chunk index). Since the hash-partitioning of the forward shuffle is not technically "stable", we need to include this local row position in the forward sort operation as well. Authors: - Richard (Rick) Zamora (https://github.com/rjzamora) Approvers: - Lawrence Mitchell (https://github.com/wence-) URL: #23524

rjzamora added 12 commits July 27, 2026 13:15

add basic shift().over() support

bb616fa

Merge remote-tracking branch 'upstream/main' into basic-shift-over

7e55c44

revise _apply_unary_op override

e8e5af2

Merge remote-tracking branch 'upstream/main' into basic-shift-over

1a2fb7e

simplify

ea8690c

use _contains_input_order_window_without_order_by

ccbb234

preserve input order and add test coverage

29396e8

Merge remote-tracking branch 'upstream/main' into preserve-order-over

4c9d125

remove unnecessary stamp

cdf69d9

Merge remote-tracking branch 'upstream/main' into preserve-order-over

6070c72

roll-back unintended changes

0a97b2f

update arg name

b4d9ed4

rjzamora self-assigned this Jul 28, 2026

rjzamora added the feature request New feature or request label Jul 28, 2026

rjzamora requested a review from a team as a code owner July 28, 2026 21:07

rjzamora added 2 - In Progress Currently a work in progress non-breaking Non-breaking change labels Jul 28, 2026

rjzamora requested a review from TomAugspurger July 28, 2026 21:07

github-actions Bot added Python Affects Python cuDF API. cudf-polars Issues specific to cudf-polars labels Jul 28, 2026

github-project-automation Bot added this to cuDF Python Jul 28, 2026

GPUtester moved this to In Progress in cuDF Python Jul 28, 2026

coderabbitai Bot reviewed Jul 28, 2026

View reviewed changes

Comment thread python/cudf_polars/tests/streaming/test_spmd.py

rjzamora mentioned this pull request Jul 28, 2026

Support shift over grouped windows in cudf-polars #23451

Merged

update test

7ea279c

mroeschke approved these changes Jul 28, 2026

View reviewed changes

Comment thread python/cudf_polars/cudf_polars/streaming/actor_graph/over.py Outdated

rjzamora added 2 commits July 28, 2026 16:09

Merge remote-tracking branch 'upstream/main' into preserve-order-over

cf3e451

in-line the sort helper

6ca710a

rjzamora removed the 2 - In Progress Currently a work in progress label Jul 28, 2026

rjzamora added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Jul 28, 2026

rapids-bot Bot merged commit 982a387 into NVIDIA:main Jul 29, 2026
110 checks passed

github-project-automation Bot moved this from In Progress to Done in cuDF Python Jul 29, 2026

rjzamora deleted the preserve-order-over branch July 29, 2026 02:40

This was referenced Jul 29, 2026

Support fixed-size rolling windows inside over() #23468

Merged

Use OrderScheme metadata to select order-aware groupby execution #23306

Merged

Support diff() inside over() in cudf-polars #23497

Merged

rjzamora mentioned this pull request Aug 4, 2026

Fix order preservation in grouped-window evaluation #23524

Merged

3 tasks

rjzamora mentioned this pull request Aug 7, 2026

[Story][FEA] Streaming ordered-window execution in cuDF-Polars #22032

Open

josephine-wolf-oberholtzer added this to cuDF Python Aug 12, 2026

josephine-wolf-oberholtzer moved this to Done in cuDF Python Aug 12, 2026

Navigation Menu

Uh oh!

Preserve input order for grouped window evaluation - #23466

Preserve input order for grouped window evaluation#23466
rapids-bot[bot] merged 15 commits into
NVIDIA:mainfrom
rjzamora:preserve-order-over

rjzamora commented Jul 28, 2026

Uh oh!

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

rjzamora commented Jul 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

rjzamora commented Jul 28, 2026

Description

Checklist

Uh oh!

coderabbitai Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rjzamora commented Jul 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading